Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration updates. #128

Merged
merged 7 commits into from
Nov 1, 2024
Merged

Configuration updates. #128

merged 7 commits into from
Nov 1, 2024

Conversation

Mpdreamz
Copy link
Member

  • RequestData is now used as a bound instance over ITransportConfiguration and IRequestConfiguration

record versions of TransportConfiguration and RequestConfiguration now exists

You can create a new instance of TransportConfiguration based off another (with test that all properties get assigned)

Ensure only RequestData deals is in charge of dealing with global and local configuration. The meant some updates to our request pipelines.

We now use a single RequestData instance if no per request overrides are provided

- RequestData is now used as a bound instance over `ITransportConfiguration` and `IRequestConfiguration`

record versions of `TransportConfiguration` and `RequestConfiguration` now exists

You can create a new instance of `TransportConfiguration` based off another (with test that all properties get assigned)

Ensure only `RequestData` deals is in charge of dealing with global and local configuration. The meant some updates to our request pipelines.

We now use a single `RequestData` instance if no per request overrides are provided
Copy link
Collaborator

@stevejgordon stevejgordon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Increased the time tolerance for the 'LastUpdate' field comparison from 100 milliseconds to 2 seconds. This change enhances the reliability of the test by accommodating larger variations in timing.
@Mpdreamz Mpdreamz merged commit e8cd3c2 into main Nov 1, 2024
5 checks passed
@Mpdreamz Mpdreamz deleted the refactor/request-data-v2 branch November 1, 2024 14:06
@Mpdreamz Mpdreamz mentioned this pull request Nov 1, 2024
Mpdreamz added a commit that referenced this pull request Nov 8, 2024
This PR continues #128 and ensures `RequestPipeline` can be shared over
many requests unless a local configuration is provided.

Similar to `RequestData` being shared in #128.

This includes further refactorings:

- `DateTimeProvider` is only provided externally once (on `NodePool`)
and that instance is used everywhere. Before it could be set seperately
on `NodePool` and `TransportConfiguration`, not by design but by
necessity.

- `RequestPipeline` is no longer disposable (we didn't actually disposed
anything).

- A new type exists `Auditor` this is now explicitly passed to the
methods that need it on `RequestPipeline`. It implements
`IReadOnlyCollection<Audit>` and is exposed as such to users.

- This PR also merges `DefaultRequestPipeline` and `RequestPipeline`
into one.
@flobernd flobernd added the v0.5.0 label Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants